-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Show all metadata and mark the locked ones with lock icon #1010
Conversation
07d4b97
to
225bab9
Compare
@@ -208,16 +242,19 @@ fun EntityMetadataItem.toMetadata(): Metadata? = when (val typed = value.typed) | |||
Metadata.Primitive( | |||
key = key, | |||
value = "${it.resourceAddress}:${it.nonFungibleId}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't here we also use NonFungibleGlobalId
as a type, same as for MetadataNonFungibleGlobalIdValue
above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops missed that.
@@ -37,23 +109,23 @@ class StateDatabaseConverters { | |||
// Behaviours | |||
@TypeConverter | |||
fun stringToBehaviours(string: String?): BehavioursColumn? { | |||
return string?.let { BehavioursColumn(behaviours = json.decodeFromString(string)) } | |||
return string?.let { json.decodeFromString(string) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we should use same json
as we use for gateway models serialization?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it is different, this is how the column is serialized into a json string. Room stores metadata in a custom type called MetadataColumn
which uses this serialisation logic inside. Also the Metadata
type we have defined is a domain model and not a GW one.
details: StateEntityDetailsResponseItemDetails?, | ||
type: ResourceEntityType, | ||
synced: Instant | ||
): ResourceEntity { | ||
val metadata = metadataCollection?.toMetadata().orEmpty() | ||
val metadataColumn = if (implicitMetadata != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the difference between explicit/implicit metadata?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Explicit metadata are the ones we request when we communicate with GW and GW ensures that we will receive them. Also known as standard metadata. For example a resource may have defined 150 metadata. If you do a simple entity details request, on the first response we will only receive the first page of them (100) and we need to paginate them in order to get the other 50.
Although we need to somehow make sure that some metadata will be received since are crucial to rendering the UI without the need of pagination, like name, symbol, description, icon_url....
So for these requests you will see that we put ExplicitMetadataKey.forAssets
in order to retrieve in the explicit_metadata
field these crucial ones we need for ui.
When the user navigates to asset details, we can also display the rest of the metadata (besides the explicit ones). Decided to call them implicit. If we know that there is a page to be paginated, this is the time we do it.
) { | ||
KeyView() | ||
MetadataKeyView(key = key, isLocked = isLocked) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it is worth to allocate some max width to the metadata key, I noticed that for some metadata names, long ones, there is not much space left for the value. When increasing font size a bit in the system (from already quite big default setting on samsung), value is pushed out of the screen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I saw that too. It is a bit tricky this one. Since we need to see if both key and value cannot be squeezed into a single row. If I added a max width to the key, and a value was too small, you would see a key being separated in two lines while there would clearly be way for the value in the same row.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jakub-rdx I pushed a commit ditching the Row
and replacing it with constraint layout. It works better know. If a key has more than 30 chars it divides the space a little better.
text = metadata.value, | ||
style = style, | ||
color = color, | ||
textAlign = if (isRenderedInNewLine) TextAlign.Start else TextAlign.End, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: maybe we can declare textAlign before when
claus once and use it in all cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not applicable in all cases though. There are cases where we align start only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works good, great work in generalizing metadata views! Only one comment about metadata key width
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested and it seems to be working fine 👌🏽
@@ -234,57 +233,47 @@ class AccountsStateCache @Inject constructor( | |||
result | |||
} | |||
|
|||
private fun Flow<MutableMap<AccountAddress, AccountCachedData>>.compileAccountAddressAssets(): Flow<List<AccountAddressWithAssets>> = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should better keep the return type, unless it is extremely obvious what it returns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's so much easier to read the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I did that but detekt complains
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I did that but detekt complains
What? 😮
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this and the other one is unused, right?
good that you removed them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and it works fine
1f79860
to
11bb869
Compare
Quality Gate failedFailed conditions |
Description
Asset details dialog got upgraded to support, non standard metadata. The metadata are received from Gateway paginated.
UI
We display the non standard metadata in the same manner we display the NF Data. Using the
MetadataView
. This view got 2 changes:1.
PublicKey
s andPublicKeyHash
es display in 1 line with...
on the end and can be copied.2. The lock icon is introduced for metadata that have
is_locked = true
3. The previously known as
AssetMetadataRow
is renamed toMetadataView
and became modular to either accept metadata or primitive values (used for standard metadata)4. The non standard metadata will be shown at the bottom with a divider.
Also the claim button in the NFT dialog has been updated.
Note
We need to clarify on UI howname
,symbol
andicon_url
will display their locked property when they are locked.The decision was to put the lock icon only in the non standard metadata.
How to test
many-metadata.txt
Videos
meta.webm
PR submission checklist